Skip to content

Fix warnings and stabilize orbit diagnostics#299

Open
krystophny wants to merge 2 commits intomainfrom
feature/fpm-bind
Open

Fix warnings and stabilize orbit diagnostics#299
krystophny wants to merge 2 commits intomainfrom
feature/fpm-bind

Conversation

@krystophny
Copy link
Copy Markdown
Member

@krystophny krystophny commented Jan 6, 2026

User description

Summary

  • clean up warning sites in orbit integration, diagnostics, samplers, and classification
  • fix cut detector stencil initialization ordering to avoid out-of-bounds access
  • adjust boozer converter imports and shift calculation to remove warnings

Testing

  • make test (timed out while running test_netcdf_results) [log: /tmp/simple_make_test_2026-01-06b.log]
  • make test TEST=test_netcdf_results [log: /tmp/simple_test_netcdf_results_2026-01-06.log]
  • make test TEST=test_engineering [log: /tmp/simple_test_engineering_2026-01-06.log]

PR Type

Bug fix, Enhancement


Description

  • Replace local dp kind definitions with intrinsic iso_fortran_env import

  • Fix cut detector stencil initialization to avoid out-of-bounds array access

  • Remove unused variables and clean up compiler warnings across modules

  • Improve code formatting and fix floating-point comparison logic


Diagram Walkthrough

flowchart LR
  A["Local dp definitions"] -->|Replace with| B["iso_fortran_env import"]
  C["Cut detector loop logic"] -->|Refactor into| D["Two-phase initialization"]
  E["Unused variables"] -->|Remove| F["Clean codebase"]
  G["Floating-point comparison"] -->|Fix| H["Proper zero check"]
Loading

File Walkthrough

Relevant files
Cleanup
2 files
simple_diag_orbit.f90
Remove local dp definition and unused variable                     
+1/-5     
diag_orbit.f90
Remove unused ierr_orbit variable declaration                       
+2/-2     
Enhancement
4 files
classification.f90
Add iso_fortran_env import, remove local dp definition     
+2/-4     
orbit_symplectic.f90
Add iso_fortran_env import, remove unused variables           
+10/-12 
samplers.f90
Add iso_fortran_env import, remove local dp definition     
+2/-4     
sub_alpha_lifetime_can.f90
Add iso_fortran_env import, improve subroutine signatures
+6/-9     
Bug fix
3 files
cut_detector.f90
Fix stencil initialization and refactor loop logic             
+77/-19 
netcdf_results_output.f90
Fix floating-point zero comparison logic                                 
+1/-1     
boozer_converter.F90
Remove unused import and fix shift calculation                     
+3/-3     

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Jan 6, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Jan 6, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Restore missing solver variables

Restore the declaration of pivot and info variables, which were removed in the
PR. These are necessary for the dgesv LAPACK solver call and their absence will
cause a compilation error.

src/orbit_symplectic.f90 [425]

 real(dp) :: fvec(n), fjac(n,n), jinv(n,n)
+integer :: pivot(n), info

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 9

__

Why: The PR incorrectly removed the pivot and info variable declarations, which are required by the dgesv LAPACK routine, causing a compilation error. This suggestion correctly restores them.

High
Remove dead code from loop

Remove the unreachable cut detection logic from the initialization loop in
advance_to_cut. The condition if(i.gt.nplagr ...) is always false within this
loop, making the code dead.

src/cut_detector.f90 [88-149]

       do i = 1, nplagr
         call tstep(si, f, z, ierr)
         if (ierr.ne.0) exit
 
         self%par_inv = self%par_inv + z(5)**2 ! parallel adiabatic invariant
 
         self%orb_sten(1:5, i) = z
         self%orb_sten(6, i) = self%par_inv
 
         !-------------------------------------------------------------------------
-        ! Tip detection and interpolation
-
+        ! Tip detection
         if(self%alam_prev.lt.0.d0.and.z(5).gt.0.d0) then
           self%itip=0   !<=tip has been passed
         end if
         self%itip=self%itip+1
         self%alam_prev=z(5)
-
-        !<=use only initialized stencil
-        if(i.gt.nplagr .and. self%itip.eq.nplagr/2) then
-          cut_type = 0
-          call plag_coeff(nplagr, nder, 0d0, self%orb_sten(5, self%ipoi), &
-            self%coef)
-          var_cut = matmul(self%orb_sten(:, self%ipoi), self%coef(0,:))
-          var_cut(2) = modulo(var_cut(2), twopi)
-          var_cut(3) = modulo(var_cut(3), twopi)
-          self%par_inv = self%par_inv - var_cut(6)
-          return
-        end if
-
-        ! End tip detection and interpolation
+        ! End tip detection
 
         !-------------------------------------------------------------------------
-        ! Periodic boundary footprint detection and interpolation
-
+        ! Periodic boundary footprint detection
         if(z(3).gt.dble(self%kper+1)*self%fper) then
           self%iper=0   !<=periodic boundary has been passed
           phiper=dble(self%kper+1)*self%fper
           self%kper=self%kper+1
         elseif(z(3).lt.dble(self%kper)*self%fper) then
           self%iper=0   !<=periodic boundary has been passed
           phiper=dble(self%kper)*self%fper
           self%kper=self%kper-1
         endif
         self%iper=self%iper+1
-
-        !<=use only initialized stencil
-        if(i.gt.nplagr .and. self%iper.eq.nplagr/2) then
-          cut_type = 1
-          !<=stencil around periodic boundary is complete, interpolate
-          call plag_coeff(nplagr, nder, 0d0, self%orb_sten(3, self%ipoi) - &
-            phiper, self%coef)
-          var_cut = matmul(self%orb_sten(:, self%ipoi), self%coef(0,:))
-          var_cut(2) = modulo(var_cut(2), twopi)
-          var_cut(3) = modulo(var_cut(3), twopi)
-          return
-        end if
-
-        ! End periodic boundary footprint detection and interpolation
+        ! End periodic boundary footprint detection
         !-------------------------------------------------------------------------
 
-  end do
+      end do
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies dead code within the first do loop, where the condition i.gt.nplagr can never be met, making the enclosed logic unreachable.

Medium
General
Use tolerance for zero check

Replace the direct comparison of floating-point values with zero with a check
against a small tolerance (e.g., 1.0e-12_dp) for robustness.

src/netcdf_results_output.f90 [321]

-zend_is_zero = all(abs(zend(1:3, i)) <= 0.0_dp)
+zend_is_zero = all(abs(zend(1:3, i)) <= 1.0e-12_dp)
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out that comparing floating-point numbers directly to zero is fragile and recommends using a small tolerance, which is a standard best practice.

Low
  • Update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant